Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick Switcher #1662

Merged
merged 19 commits into from
Feb 27, 2024
Merged

Quick Switcher #1662

merged 19 commits into from
Feb 27, 2024

Conversation

juligasa
Copy link
Collaborator

Implementation of the quick switcher

@juligasa juligasa self-assigned this Feb 20, 2024
2024-02-23.01
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, don’t touch this file! @juligasa

This folder contains the “baseline” database which is the first ever schema we support migrations for. So on top of this database migrations are applied in a test to verify that the migrated schema matches the desired schema defined in schema.sql file.

Basically, first you want to change schema.sql file to the new desired schema, and then create the necessary migration to move from the previous schema to the desired schema.

Copy link
Collaborator

@burdiyan burdiyan Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment here for more info: https://github.com/MintterHypermedia/mintter/blob/b93b795e45b0fb0f180249d9558dd23462265250/backend/daemon/storage/migrations_test.go#L19

I now realize that this was not the most obvious place to find this information (should find a better home for it 😁).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes I read through that but since I was getting an error in the tests saying

--- Expected
                                +++ Actual
                                @@ -102,4 +102,5 @@
                                     author INTEGER REFERENCES public_keys (id),
                                -    resource INTEGER REFERENCES resources (id)
                                -, meta TEXT) WITHOUT ROWID
                                +    resource INTEGER REFERENCES resources (id),
                                +    meta TEXT
                                +) WITHOUT ROWID
                                 CREATE INDEX structural_blobs_by_author ON structural_blobs (author, resource) WHERE author IS NOT NULL

I though that I should update the Golden, But you are right is the baseline. The test should pass. However the reason why it does not pass seems like the generated schema after the ALTER table is not correct. I will drop tables and indexes and create them from scratch like in previous migrations.

@burdiyan
Copy link
Collaborator

@juligasa we might want to index multiple fields on groups. For example url in addition to the title. What if we use this new “meta” column to store multiple fields as JSON, and then use SQLite’s JSON extension to access those fields to perform the match. Or even just treat them as raw text for fuzzy matching.

{Version: "2024-02-23.01", Run: func(_ *Dir, conn *sqlite.Conn) error {
return sqlitex.ExecScript(conn, sqlfmt(`
DROP TABLE IF EXISTS structural_blobs;
DROP INDEX IF EXISTS structural_blobs_by_author;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juligasa you don't need to drop indexes if you drop the parent table.

@ericvicenti ericvicenti merged commit e7b7eeb into main Feb 27, 2024
2 checks passed
@ericvicenti ericvicenti deleted the feat/quick-switcher branch February 27, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants